Track panic
in Unit early.
#6170
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternate solution for #5445. It ensures that
panic
is cleared in the Profile for "for_host" targets (proc-macro/plugin/build-scripts) and dependencies. This helps avoid unnecessary recompiles in some situations (though does add extra units in some situations, see below).Some examples where extra rebuilds are now avoided:
build --all
should build everything, and thenbuild -p foo
was causing a recompile because the shared dep was no longer in theused_in_plugin
set. Now it should not recompile.panic=abort
, with a shared dependency in build and dev,build
would cause that shared dependency to be built twice (exactly the same without panic set). Now it should only build once.Some examples where
panic
is now set correctly:panic=abort
, with a binary with a shared dependency in build and normal,test
would cause that shared dependency to be built twice (exactly the same without panic set). Now it is still built twice, but the one for the normal (bin) dependency will correctly havepanic
set.Some examples where new units are now generated:
panic=abort
, with shared dependency between normal and proc-macro or build. Previously the shared dependency was built once withpanic=unwind
. Now it is built separately (once withpanic
, once without). I feel like that this is more correct behavior — that now the normal dependency avoids adding landing pads.For
panic=abort
cross-compiling, this makes no difference in compile time since it was already built twice.Additional notes:
panic
is cleared for it and all its dependencies. There could be arguments made to change that (Shouldbuild.rs
be compiled like plugins? #5436), but it doesn't seem important to me.ProfileFor
toUnitFor
. I expect this API to continue to evolve in the future.Closes #6143, closes #6154.